Skip to content

Fix DenonAvr discovery of Denon DN-500AV#37529

Merged
bdraco merged 8 commits intohome-assistant:devfrom
starkillerOG:patch-32
Jul 7, 2020
Merged

Fix DenonAvr discovery of Denon DN-500AV#37529
bdraco merged 8 commits intohome-assistant:devfrom
starkillerOG:patch-32

Conversation

@starkillerOG
Copy link
Copy Markdown
Contributor

@starkillerOG starkillerOG commented Jul 5, 2020 •
edited by bdraco
Loading

Proposed change

Fix DenonAvr discovery of Denon DN-500AV

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

Config Flow

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@probot-home-assistant
Copy link
Copy Markdown

probot-home-assistant Bot commented Jul 5, 2020

Hey there @scarface-4711, mind taking a look at this pull request as its been labeled with an integration (denonavr) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@starkillerOG
Copy link
Copy Markdown
Contributor Author

starkillerOG commented Jul 5, 2020 •
edited
Loading

To fully fix this issue, this PR in the parent lib needs to be merged: ol-iver/denonavr#145
The required PR is now merged

@bdraco bdraco added the waiting-for-upstream We're waiting for a change upstream label Jul 5, 2020
@starkillerOG starkillerOG changed the title DenonAvr: fix discovery of Denon DN-500AV Fix DenonAvr discovery of Denon DN-500AV Jul 5, 2020
@frenck frenck marked this pull request as draft July 5, 2020 18:58
@frenck
Copy link
Copy Markdown
Member

frenck commented Jul 5, 2020

Converted the PR to draft until upstream is ready.

@ol-iver
Copy link
Copy Markdown
Contributor

ol-iver commented Jul 6, 2020

@starkillerOG there is version 0.9.4 which includes your fix now

@starkillerOG starkillerOG marked this pull request as ready for review July 6, 2020 19:42
@starkillerOG
Copy link
Copy Markdown
Contributor Author

starkillerOG commented Jul 6, 2020

@bdraco how can I remove the waiting-for-upstream label, or can that only be done by maintainers?

@bdraco bdraco removed the waiting-for-upstream We're waiting for a change upstream label Jul 6, 2020
@starkillerOG
Copy link
Copy Markdown
Contributor Author

starkillerOG commented Jul 6, 2020

Test failure is unrelated, the problem is in this file:
tests/components/unifi/test_device_tracker.py

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Jul 7, 2020

Tested on my production install. All ok

@bdraco bdraco merged commit 3f1d25b into home-assistant:dev Jul 7, 2020
@bdraco bdraco added this to the 0.112.4 milestone Jul 7, 2020
balloob pushed a commit that referenced this pull request Jul 9, 2020
@starkillerOG @balloob
Fix DenonAvr discovery of Denon DN-500AV (#37529)
8f77cc2 
* fix discovery of Denon DN-500AV

* fix discovery of Denon DN-500AV

* fix discovery of Denon DN-500AV

* fix typo

* bump denonavr to 0.9.4

* add ignored model test

* fix mistake in branch
@balloob balloob mentioned this pull request Jul 9, 2020
Merged
@netforceatg
Copy link
Copy Markdown

netforceatg commented Jul 31, 2020

Running Home Assistant 0.113.2

Have used the denonavr component to control my Zone 2 of my Denon AVR-X5200W for a few years. Been really happy with it

However with the updates, I have removed the denonavr from configuration.yaml and added it through the web intergration. However HA does not find it on the IP i put in(local browser on my computer finds the denon webpage control fine).

Any advice? (I know the avr have network connectivity)

image

image

@starkillerOG
Copy link
Copy Markdown
Contributor Author

starkillerOG commented Jul 31, 2020

Have you read the error message?
Disconnects mains power of the receiver wait 10 seconds, plug it back in

@netforceatg
Copy link
Copy Markdown

netforceatg commented Jul 31, 2020

Thanks, I've leaved the reciever unplugged and retested. Same result.

Here is the errorlog:
2020-07-31 19:36:23 ERROR (ThreadPoolExecutor-6_0) [DenonSSDP] Error occurred during evaluation of SCPD XML: not well-formed (invalid token): line 24, column 79
2020-07-31 19:36:23 ERROR (ThreadPoolExecutor-7_1) [DenonAVR] Host 192.168.2.15 returned malformed XML for end point /goform/formMainZone_MainZoneXml.xml: not well-formed (invalid token): line 3, column 84
2020-07-31 19:36:23 ERROR (ThreadPoolExecutor-6_1) [DenonAVR] Host 192.168.2.15 returned malformed XML for end point /goform/formMainZone_MainZoneXml.xml: not well-formed (invalid token): line 3, column 84
2020-07-31 19:36:23 WARNING (ThreadPoolExecutor-6_1) [DenonAVR] Receiver name could not be determined. Using standard name: Denon AVR.
2020-07-31 19:36:23 ERROR (ThreadPoolExecutor-8_2) [DenonAVR] Host 192.168.2.15 returned malformed XML for end point /goform/formMainZone_MainZoneXml.xml: not well-formed (invalid token): line 3, column 84
2020-07-31 19:36:23 ERROR (MainThread) [homeassistant.components.denonavr.receiver] Missing receiver information: manufacturer 'None', name 'Denon AVR', model 'None', type 'avr-x'
2020-07-31 19:36:32 ERROR (SyncWorker_4) [homeassistant.components.scrape.sensor] Unable to extract data from HTML
2020-07-31 19:36:45 ERROR (MainThread) [frontend.js.latest.202007160] http://192.168.2.30:83/frontend_latest/chunk.cf565e41b8a8dce0642a.js:133:238 Uncaught TypeError: Cannot read property 'header' of null
2020-07-31 19:36:46 ERROR (MainThread) [frontend.js.latest.202007160] http://192.168.2.30:83/frontend_latest/chunk.cf565e41b8a8dce0642a.js:133:238 Uncaught TypeError: Cannot read property 'header' of null
2020-07-31 19:36:47 ERROR (MainThread) [frontend.js.latest.202007160] http://192.168.2.30:83/frontend_latest/chunk.cf565e41b8a8dce0642a.js:133:238 Uncaught TypeError: Cannot read property 'header' of null
2020-07-31 19:37:02 ERROR (SyncWorker_4) [homeassistant.components.scrape.sensor] Unable to extract data from HTML

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Jul 31, 2020

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jul 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bdraco bdraco bdraco approved these changes

Assignees

No one assigned

Labels

bugfix by-code-owner cherry-picked cla-signed integration: denonavr small-pr PRs with less than 30 lines.

Projects

None yet

Milestone

0.112.4

Development

Successfully merging this pull request may close these issues.

Denon AVR is not being detected by the config flow

8 participants

@starkillerOG @frenck @ol-iver @bdraco @netforceatg @MartinHjelmare @balloob @homeassistant